fix(gl): sign the CLI's /ipfs/pins reads under the #134 auth gate#146
Open
beardthelion wants to merge 6 commits into
Open
fix(gl): sign the CLI's /ipfs/pins reads under the #134 auth gate#146beardthelion wants to merge 6 commits into
beardthelion wants to merge 6 commits into
Conversation
…gate /api/v1/ipfs/pins now 401s anonymous callers (#134). Load the caller's identity (--dir, default keystore) and use NodeClient::get_signed; when no identity exists, propagate load_keypair_from_dir's existing 'gl identity new' error instead of a raw 401. Tests: signed-headers present, empty, and no-identity-issues-no-request (mockito).
The pins panel signs its /api/v1/ipfs/pins read (#134 gates it behind auth) via an injectable fetch_pins helper with four states: pins, empty, unavailable (signed read rejected/errored), and needs-identity (no keypair, no request issued). cmd_status loads the identity gracefully so a missing keystore never aborts status; peers/repos/p2p/events panels stay anonymous. Tests drive fetch_pins directly against a mock node.
…el label - gl ipfs list: check the /ipfs/pins response status before parsing, so a rejected signed read (e.g. 401) surfaces as an error instead of silently printing 'No IPFS pins recorded'. Matches the sibling gl ipfs get. - gl node status: PinsPanel::Empty now renders 'Pinned CIDs: 0' (a reachable node with zero pins), reserving 'unavailable' for the failure state; the prior label reused 'IPFS not configured' and misreported the zero-pins case. - Carry the resolved count in PinsPanel::Pins, removing the duplicated count-fallback closure between fetch_pins and the render arm.
Extract the pins-panel render into a testable pins_status_line helper and assert all four states' output (closes the untested cmd_status render path, including the empty->'Pinned CIDs: 0' label). Add fetch_pins tests for the two error branches that were code-traced but unexecuted: a malformed 200 body and a transport error both degrade to Unavailable without panicking.
beardthelion
added
crate:attest
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (183)
💤 Files with no reviewable changes (81)
📝 WalkthroughWalkthroughThis PR deletes nearly the entire repository, removing all Rust crates (gitlawb-core, gitlawb-node, gitlawb-attest, git-remote-gitlawb), CI/CD workflows, GitHub configuration files, issue/PR templates, licenses, documentation, and configuration files, leaving no replacement content. Changes
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
beardthelion
force-pushed
the
fix/gl-sign-ipfs-pins
branch
from
ec367bc to
93ab3da
Compare
jatmn
requested changes
Jul 2, 2026
jatmn
left a comment
jatmn
left a comment
Collaborator
There was a problem hiding this comment.
I found an issue that needs to be addressed before this is ready.
Findings
- [P2] Let
gl node statususe the same identity directory as the signed pins list
crates/gl/src/node.rs:22
This PR adds--dirtogl ipfs listand signs that request with the selected identity, but the other pins caller still has no identity-directory option:gl node status --helponly exposes--node, and the implementation always callsload_keypair_from_dir(None). Users who created or selected an identity with--dircan makegl ipfs list --dir ...work, but the status dashboard will still render the pins panel as “sign in to view” because it cannot load that same key. Please add adiroption toNodeCmd::Statusand pass it into the pins fetch path so both CLI callers can authenticate consistently.
`gl node status` always loaded the default keystore via `load_keypair_from_dir(None)`, so a user who created or selected an identity with `--dir` saw the status pins panel render "sign in to view" even though `gl ipfs list --dir` authenticated fine. Add a `--dir` option to `NodeCmd::Status` and thread it into the signed pins fetch, so both CLI pins callers select the same identity directory. An explicit `--dir` that can't be loaded (missing, corrupt, or unreadable) no longer masquerades as "no identity, run `gl identity new`": the pins identity is resolved up front into Keyed / Anonymous / DirUnusable, and the unusable explicit-dir case renders a distinct "no usable identity in <dir>" panel line that names the path, while a missing default keystore (no `--dir`) still degrades quietly. A pins failure never aborts the dashboard. Tests cover both flag branches (Some/None) and the resolver both ways: a good `--dir` loads Keyed, an unusable one reports DirUnusable/IdentityError without issuing a request. The new symbols were absent before, so they compile-RED first.
beardthelion
force-pushed
the
fix/gl-sign-ipfs-pins
branch
from
9640c87 to
cf588d0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR #134 gates
/api/v1/ipfs/pinsbehind authentication (anonymous callers get 401). TwoglCLI call sites still hit it unsigned, so once #134 ships they break:gl ipfs listreturns a hard 401, and thegl node statuspins panel silently shows nothing. This signs both reads with the caller's identity through the existingNodeClient::get_signed, so the CLI keeps working under the gate.Scope is pins-only:
gl ipfs listnow loads the identity (via--dir, defaulting to the standard keystore) and signs the request. With no identity it fails with the existinggl identity newguidance instead of a raw 401, and it now surfaces a non-2xx response as an error rather than parsing it into an empty list.gl node statusloads the identity gracefully and signs only the pins panel, via an injectablefetch_pinshelper with four states (pins, empty, unavailable, sign-in-required). The peers, repos, p2p, and events panels stay anonymous, and a pins failure never aborts the dashboard./api/v1/arweave/anchors(the other endpoint #134 gates) is never called by the CLI, so it is out of scope.A note on behavior: the pin listing is visibility-filtered (every repo the caller can read, public plus their own private), not scoped to the caller's own objects. The auth requirement exists to stop anonymous enumeration of the node-wide pin index, so the CLI has to authenticate, but the result is the caller's readable view.
This should land in the same release as #134 so no published build ships the broken CLI.
Tests are mockito-based (no database): signed-header presence, the no-identity and non-2xx error paths, and all four node-status pins states including the transport-error and malformed-body branches. The
glsuite passes (236 tests).Closes the CLI regression noted in the #134 review.
Summary by CodeRabbit